Reviewed 8 of 10 files at r1.
Reviewable status: 7 of 10 files reviewed, 3 unresolved discussions (waiting on @dlon)

.github/workflows/desktop-e2e.yml line 248 at r1 (raw file):

        with:
          token: ${{ secrets.GITHUB_TOKEN }}
          node-version: 20

We should use the node-version-file in this workflow as well! This workflow must have been forgotten when we switched to Volta 🤔

.github/workflows/translations.yml line 29 at r1 (raw file):

        uses: actions/setup-node@v4
        with:
          node-version: ${{ steps.volta.outputs.node }}

We should use the node-version-file here as well!

gui/standalone-tests.ts line 79 at r1 (raw file):

  if (process.platform === 'darwin') {
    child_process.spawnSync('/usr/bin/codesign', ['--sign', '-', nodeBin]);

Do we really need to sign it? When I ran the hello example on my mac it worked fine without a signature?

Reviewable status: 6 of 10 files reviewed, 4 unresolved discussions (waiting on @raksooo)

.github/workflows/translations.yml line 29 at r1 (raw file):

Done.

.github/workflows/translations.yml line 34 at r2 (raw file):

      - name: Update NPM
        run: npm i -g npm@${{ steps.volta.outputs.npm }}

Do you know why we explicitly updated npm here? I suspect it's unnecessary, but I might be wrong.

gui/standalone-tests.ts line 79 at r1 (raw file):

I had to sign it. Otherwise, macOS would kill the process. I think this requirement is stricter on arm Macs.

Reviewable status: 6 of 10 files reviewed, 4 unresolved discussions (waiting on @raksooo)

.github/workflows/desktop-e2e.yml line 248 at r1 (raw file):

Done.

.github/workflows/translations.yml line 34 at r2 (raw file):

It might be that we were using the wrong npm (somewhere in PATH). shell: bash seemed to fix this.

Reviewed 3 of 3 files at r3, 2 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dlon)

.github/workflows/translations.yml line 34 at r2 (raw file):

I'm pretty sure we updated NPM because we don't always use the npm version corresponding to the Node version installed. But maybe that is solved automatically now with node-version-file.

gui/standalone-tests.ts line 79 at r1 (raw file):

Okay 👍

gui/scripts/build-test-executable.sh line 29 at r5 (raw file):

    local temp_output="./build/temp-test-executable$bin_suffix"
    local output="../dist/app-e2e-tests-$PRODUCT_VERSION-$TARGET$bin_suffix"
    local node_copy_path="./build/test/node$bin_suffix"

We probably shouldn't put anything like node in the build directory. I think it will be packaged into the Electron app by electron-builder 🤔

Reviewable status: 9 of 10 files reviewed, 2 unresolved discussions (waiting on @raksooo)

gui/scripts/build-test-executable.sh line 29 at r5 (raw file):

Done.

Reviewable status: 9 of 10 files reviewed, 2 unresolved discussions (waiting on @dlon)

gui/scripts/build-test-executable.sh line 38 at r6 (raw file):

    cp "$node_path" "$node_copy_path"
    # shellcheck disable=SC2068
    tar -czf ./build/test/assets.tar.gz ${ASSETS[@]}

This should go in $temp_dir as well right?

Reviewable status: 9 of 10 files reviewed, 2 unresolved discussions (waiting on @raksooo)

gui/scripts/build-test-executable.sh line 38 at r6 (raw file):

Preferably not, since the asset path needs to be specified in standalone-tests.sea.json.

Reviewed 1 of 1 files at r6, 1 of 1 files at r7, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dlon)

Reviewed all commit messages.
Reviewable status: 9 of 10 files reviewed, 1 unresolved discussion (waiting on @dlon and @raksooo)